Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

in get_structured_config_data: check for dict subclass data #653

Merged
merged 8 commits into from
Apr 11, 2021

Conversation

Jasha10
Copy link
Collaborator

@Jasha10 Jasha10 commented Mar 30, 2021

Closes #584

TODO:

  • add a test case to test_errors.py for the new try: ... except ValidationError: ... code path.
  • add bugfix news fragment

Comment on lines 907 to 911
def test_dict_subclass_data_preserved_upon_node_creation(self, module: Any) -> None:
src = module.DictSubclass.Str2Str()
src["baz"] = "qux"
cfg = OmegaConf.structured(src)
assert cfg.baz == "qux"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If run against the master branch, the above test raises ConfigAttributeError: Missing key baz.
This PR addresses the error by inspecting for dict subclass data within the get_attr_data and get_dataclass_data functions.

@Jasha10 Jasha10 marked this pull request as draft March 30, 2021 02:43
omegaconf/_utils.py Outdated Show resolved Hide resolved
Copy link
Owner

@omry omry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@omry
Copy link
Owner

omry commented Apr 8, 2021

  1. make sure your todo is addressed (in the bug description).
  2. Consider adding a bugfix news fragment (if this is a bug in 2.0).

Copy link
Owner

@omry omry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like there is a new conflict.

@Jasha10 Jasha10 marked this pull request as ready for review April 9, 2021 23:09
@Jasha10
Copy link
Collaborator Author

Jasha10 commented Apr 9, 2021

Should be ready to go :)

@Jasha10 Jasha10 requested a review from omry April 11, 2021 18:04
@Jasha10 Jasha10 merged commit 9b037f9 into omry:master Apr 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dict subclass data not preserved during assignment
2 participants